feat: move onboarding prompt to enable screen #2737
Conversation
|
@reneaaron I think this is a good idea. Would @pavanjoshi914 work on it? |
|
@pavanjoshi914 After discussing this with @bumi we think it would be best to create separate enable screens (I've updated the issue description) |
| if (fetchedAccount.nostrEnabled) { | ||
| setAccountComponent(<Enable origin={props.origin} />); | ||
| } else { | ||
| setAccountComponent(<Onboard />); |
There was a problem hiding this comment.
is it good to set this as a state or should we do this in the render block depending on the state variables that is set?
There was a problem hiding this comment.
now we return component directly based on the state variable
| @@ -85,7 +85,7 @@ const routes = { | |||
| public: { | |||
| webbtc: { | |||
| onboard: onboard.prompt, | |||
| enable: allowances.enable, | |||
| enable: allowances.mnemonicEnable, | |||
There was a problem hiding this comment.
move those under the webbtc/nostr/etc. namespaces then we also indicated there that those are called from the window providers
There was a problem hiding this comment.
moved enable prompts to their respective namespaces
added separate component for each provider screen |
| @@ -4,7 +4,7 @@ import state from "~/extension/background-script/state"; | |||
| import { allowanceFixture } from "~/fixtures/allowances"; | |||
| import type { DbAllowance, MessageAllowanceEnable, Sender } from "~/types"; | |||
|
|
|||
| import enableAllowance from "../enable"; | |||
| import enableAllowance from "../../webln/enable"; | |||
There was a problem hiding this comment.
this seems wrong related to the filename. Now do we need tests for each provider?
There was a problem hiding this comment.
what shall be do here?
There was a problem hiding this comment.
this shall be not in allowance anymore. separate file in each provider?
modify contents in the screen clean up blocklist, allowance and enable functions in the provider signed-off-by: pavan joshi <pavanj914@gmail.com>
| isUnlocked && | ||
| allowance && | ||
| allowance.enabled && | ||
| account?.nostrPrivateKey |
There was a problem hiding this comment.
why is the nostrPrivateKey check needed?
There was a problem hiding this comment.
That's to force onboarding in the enable prompt, right?
There was a problem hiding this comment.
this is needed because if allowance is set we directly return from the prompt itself. we don't go to screen
There was a problem hiding this comment.
we need to open enable screen and render screen conditionally
Describe the changes you have made in this PR